feat(a2a): added a2a protocol #2784
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Greptile OverviewGreptile SummaryComprehensive A2A (Agent-to-Agent) protocol v0.3 implementation enabling Sim Studio workflows to be exposed as discoverable agents and interact with external A2A-compatible agents. Key ChangesCore Protocol Implementation:
Database Schema:
Push Notifications:
UI Improvements:
Utilities:
Issues Found
Confidence Score: 4/5
Important Files ChangedFile Analysis
Sequence DiagramsequenceDiagram
participant Client as External A2A Client
participant Server as A2A Serve Endpoint
participant Redis as Redis (Optional)
participant DB as PostgreSQL
participant Workflow as Workflow Execute
participant Trigger as Trigger.dev (Optional)
participant Webhook as Client Webhook
Note over Client,Server: Agent Discovery
Client->>Server: GET /api/a2a/serve/{agentId}
Server->>Redis: Check cache
alt Cache hit
Redis-->>Server: Return cached agent card
else Cache miss
Server->>DB: Query a2a_agent table
Server->>Redis: Cache agent card (1h TTL)
end
Server-->>Client: Agent Card (JSON)
Note over Client,Server: Message Send (Non-Streaming)
Client->>Server: POST /api/a2a/serve/{agentId}<br/>(JSON-RPC message/send)
Server->>DB: Check authentication & agent status
Server->>Redis: Acquire distributed lock
alt Lock acquired
Server->>DB: Create/update a2a_task
Server->>Workflow: POST /api/workflows/{id}/execute
Workflow-->>Server: Execution result
Server->>DB: Update task with result & artifacts
Server->>Redis: Release lock
alt Push notification configured
Server->>Trigger: Queue notification task
Trigger->>Webhook: POST task update
end
Server-->>Client: Task result (JSON-RPC)
else Lock failed
Server-->>Client: Error: Task being processed
end
Note over Client,Server: Message Stream
Client->>Server: POST /api/a2a/serve/{agentId}<br/>(JSON-RPC message/stream)
Server->>Redis: Acquire distributed lock
Server->>DB: Create/update task
Server->>Workflow: POST execute (streaming)
Server-->>Client: SSE: status event (working)
loop Streaming chunks
Workflow-->>Server: Chunk
Server-->>Client: SSE: message event
end
Workflow-->>Server: Final result
Server->>DB: Update task (completed)
Server->>Redis: Release lock
Server->>Trigger: Queue push notification
Server-->>Client: SSE: status event (completed)
Trigger->>Webhook: POST task update
Note over Client,Server: Task Operations
Client->>Server: POST tasks/get
Server->>DB: Query a2a_task
Server-->>Client: Task with history
Client->>Server: POST tasks/cancel
Server->>DB: Update task status (canceled)
Server->>Trigger: Queue push notification
Server-->>Client: Canceled task
Client->>Server: POST tasks/resubscribe
Server->>DB: Check task status
loop Polling (every 3s)
Server->>DB: Query task updates
Server-->>Client: SSE: status updates
end
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| const lockValue = uuidv4() | ||
| const acquired = await acquireLock(lockKey, lockValue, 60) | ||
|
|
||
| if (!acquired) { | ||
| return NextResponse.json( | ||
| createError(id, A2A_ERROR_CODES.INTERNAL_ERROR, 'Task is currently being processed'), | ||
| { status: 409 } | ||
| ) |
There was a problem hiding this comment.
Race condition: If two requests arrive simultaneously for the same task, both could pass the acquireLock check before either updates the database. While Redis locking is attempted, when Redis is unavailable (line 92 in redis.ts returns true), the lock is not actually acquired.
Without Redis, concurrent requests could:
- Both read the same task state
- Both attempt to execute the workflow
- Both update the task with different execution results
This is particularly problematic for terminal state checks. Consider adding a database-level check after acquiring the lock or implementing optimistic locking with a version field in the task table.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/sim/app/api/a2a/serve/[agentId]/route.ts
Line: 298:305
Comment:
Race condition: If two requests arrive simultaneously for the same task, both could pass the `acquireLock` check before either updates the database. While Redis locking is attempted, when Redis is unavailable (line 92 in redis.ts returns `true`), the lock is not actually acquired.
Without Redis, concurrent requests could:
1. Both read the same task state
2. Both attempt to execute the workflow
3. Both update the task with different execution results
This is particularly problematic for terminal state checks. Consider adding a database-level check after acquiring the lock or implementing optimistic locking with a version field in the task table.
How can I resolve this? If you propose a fix, please make it concise.| if (isTerminalState(finalState)) { | ||
| notifyTaskStateChange(taskId, finalState).catch((err) => { | ||
| logger.error('Failed to trigger push notification', { taskId, error: err }) | ||
| }) | ||
| } |
There was a problem hiding this comment.
The push notification is triggered with notifyTaskStateChange but errors are only logged and not handled. If the push notification delivery fails silently, external clients won't receive task completion updates.
While this is acceptable for fire-and-forget notifications, consider:
- At least tracking failed notification attempts in metrics/logs for monitoring
- Adding a retry mechanism for critical notifications
- Documenting that push notifications are best-effort delivery
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/sim/app/api/a2a/serve/[agentId]/route.ts
Line: 414:418
Comment:
The push notification is triggered with `notifyTaskStateChange` but errors are only logged and not handled. If the push notification delivery fails silently, external clients won't receive task completion updates.
While this is acceptable for fire-and-forget notifications, consider:
1. At least tracking failed notification attempts in metrics/logs for monitoring
2. Adding a retry mechanism for critical notifications
3. Documenting that push notifications are best-effort delivery
How can I resolve this? If you propose a fix, please make it concise.| } | ||
|
|
||
| const messageContent = finalContent || accumulatedContent || 'Task completed' | ||
| const agentMessage = createAgentMessage(messageContent) | ||
| agentMessage.taskId = taskId | ||
| if (contextId) agentMessage.contextId = contextId | ||
| history.push(agentMessage) | ||
|
|
||
| await db | ||
| .update(a2aTask) | ||
| .set({ | ||
| status: 'completed', | ||
| messages: history, | ||
| completedAt: new Date(), | ||
| updatedAt: new Date(), | ||
| }) | ||
| .where(eq(a2aTask.id, taskId)) |
There was a problem hiding this comment.
In the streaming handler, when parsing SSE chunks, if parsed.finalContent is available but empty, it will overwrite the accumulatedContent. This could result in losing streamed content if the final event has an empty content field.
| } | |
| const messageContent = finalContent || accumulatedContent || 'Task completed' | |
| const agentMessage = createAgentMessage(messageContent) | |
| agentMessage.taskId = taskId | |
| if (contextId) agentMessage.contextId = contextId | |
| history.push(agentMessage) | |
| await db | |
| .update(a2aTask) | |
| .set({ | |
| status: 'completed', | |
| messages: history, | |
| completedAt: new Date(), | |
| updatedAt: new Date(), | |
| }) | |
| .where(eq(a2aTask.id, taskId)) | |
| const messageContent = finalContent || accumulatedContent || 'Task completed' |
Consider: const messageContent = (finalContent !== undefined ? finalContent : accumulatedContent) || 'Task completed' to properly handle empty strings vs undefined.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/sim/app/api/a2a/serve/[agentId]/route.ts
Line: 646:662
Comment:
In the streaming handler, when parsing SSE chunks, if `parsed.finalContent` is available but empty, it will overwrite the `accumulatedContent`. This could result in losing streamed content if the final event has an empty content field.
```suggestion
const messageContent = finalContent || accumulatedContent || 'Task completed'
```
Consider: `const messageContent = (finalContent !== undefined ? finalContent : accumulatedContent) || 'Task completed'` to properly handle empty strings vs undefined.
How can I resolve this? If you propose a fix, please make it concise.| const pollInterval = 3000 // 3 seconds (reduced from 1s to lower DB load) | ||
| const maxPolls = 100 // 5 minutes max (100 * 3s = 300s) | ||
|
|
There was a problem hiding this comment.
The polling mechanism in handleTaskResubscribe uses a 3-second interval with a maximum of 100 polls (5 minutes total). However, there's no graceful degradation if the client disconnects early.
The cancel() method is implemented (line 1031-1037), but consider adding a connection check or using request.signal to detect early disconnection and stop polling immediately to reduce unnecessary database load.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/sim/app/api/a2a/serve/[agentId]/route.ts
Line: 933:935
Comment:
The polling mechanism in `handleTaskResubscribe` uses a 3-second interval with a maximum of 100 polls (5 minutes total). However, there's no graceful degradation if the client disconnects early.
The `cancel()` method is implemented (line 1031-1037), but consider adding a connection check or using `request.signal` to detect early disconnection and stop polling immediately to reduce unnecessary database load.
How can I resolve this? If you propose a fix, please make it concise.| * @returns The workspace if found, null otherwise | ||
| */ | ||
| export async function getWorkspaceById(workspaceId: string): Promise<WorkspaceBasic | null> { | ||
| const [ws] = await db | ||
| .select({ id: workspace.id }) | ||
| .from(workspace) | ||
| .where(eq(workspace.id, workspaceId)) | ||
| .limit(1) |
There was a problem hiding this comment.
The getWorkspaceById function only selects the id field, which is already provided as input. This seems redundant - the function is essentially checking existence.
Consider renaming this to workspaceExists and returning a boolean directly, or if you need the workspace object for future use, document why only the ID is selected:
| * @returns The workspace if found, null otherwise | |
| */ | |
| export async function getWorkspaceById(workspaceId: string): Promise<WorkspaceBasic | null> { | |
| const [ws] = await db | |
| .select({ id: workspace.id }) | |
| .from(workspace) | |
| .where(eq(workspace.id, workspaceId)) | |
| .limit(1) | |
| /** | |
| * Check if a workspace exists by ID | |
| * | |
| * @param workspaceId - The workspace ID to check | |
| * @returns True if workspace exists, false otherwise | |
| */ | |
| export async function workspaceExists(workspaceId: string): Promise<boolean> { | |
| const [ws] = await db | |
| .select({ id: workspace.id }) | |
| .from(workspace) | |
| .where(eq(workspace.id, workspaceId)) | |
| .limit(1) | |
| return !!ws | |
| } |
Note: There's already a workspaceExists function at line 59-62, but it calls getWorkspaceById which seems circular.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/sim/lib/workspaces/permissions/utils.ts
Line: 26:33
Comment:
The `getWorkspaceById` function only selects the `id` field, which is already provided as input. This seems redundant - the function is essentially checking existence.
Consider renaming this to `workspaceExists` and returning a boolean directly, or if you need the workspace object for future use, document why only the ID is selected:
```suggestion
/**
* Check if a workspace exists by ID
*
* @param workspaceId - The workspace ID to check
* @returns True if workspace exists, false otherwise
*/
export async function workspaceExists(workspaceId: string): Promise<boolean> {
const [ws] = await db
.select({ id: workspace.id })
.from(workspace)
.where(eq(workspace.id, workspaceId))
.limit(1)
return !!ws
}
```
Note: There's already a `workspaceExists` function at line 59-62, but it calls `getWorkspaceById` which seems circular.
<sub>Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!</sub>
How can I resolve this? If you propose a fix, please make it concise.| export const a2aPushNotificationTask = task({ | ||
| id: 'a2a-push-notification-delivery', | ||
| retry: { | ||
| maxAttempts: 5, | ||
| minTimeoutInMs: 1000, | ||
| maxTimeoutInMs: 60000, | ||
| factor: 2, | ||
| }, | ||
| run: async (params: A2APushNotificationParams) => { |
There was a problem hiding this comment.
The Trigger.dev task configuration has a maximum of 5 retry attempts with exponential backoff (factor: 2), which means:
- Attempt 1: immediate
- Attempt 2: ~1s later
- Attempt 3: ~2s later
- Attempt 4: ~4s later
- Attempt 5: ~8s later
Total: ~15 seconds of retries. For webhook deliveries, this might be too aggressive if the recipient server is temporarily down. Consider:
- Increasing maxTimeoutInMs to allow longer delays between retries
- Reducing maxAttempts but with longer delays
- Adding jitter to prevent thundering herd if many notifications fail simultaneously
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/sim/background/a2a-push-notification-delivery.ts
Line: 13:21
Comment:
The Trigger.dev task configuration has a maximum of 5 retry attempts with exponential backoff (factor: 2), which means:
- Attempt 1: immediate
- Attempt 2: ~1s later
- Attempt 3: ~2s later
- Attempt 4: ~4s later
- Attempt 5: ~8s later
Total: ~15 seconds of retries. For webhook deliveries, this might be too aggressive if the recipient server is temporarily down. Consider:
1. Increasing maxTimeoutInMs to allow longer delays between retries
2. Reducing maxAttempts but with longer delays
3. Adding jitter to prevent thundering herd if many notifications fail simultaneously
<sub>Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!</sub>
How can I resolve this? If you propose a fix, please make it concise.| const acquired = await acquireLock(lockKey, lockValue, 300) // 5 minute timeout for streaming | ||
|
|
||
| if (!acquired) { | ||
| const encoder = new TextEncoder() | ||
| const errorStream = new ReadableStream({ | ||
| start(controller) { | ||
| controller.enqueue( | ||
| encoder.encode( | ||
| `event: error\ndata: ${JSON.stringify({ code: A2A_ERROR_CODES.INTERNAL_ERROR, message: 'Task is currently being processed' })}\n\n` | ||
| ) | ||
| ) | ||
| controller.close() | ||
| }, | ||
| }) | ||
| return new NextResponse(errorStream, { headers: SSE_HEADERS }) | ||
| } |
There was a problem hiding this comment.
When the streaming lock cannot be acquired (line 486), an error stream is created and returned. However, the lock timeout for streaming is set to 300 seconds (5 minutes, line 484), which is quite long.
If a streaming request hangs or fails without properly releasing the lock, subsequent streaming requests for the same task will be blocked for up to 5 minutes. Consider:
- Using a shorter lock timeout for streaming operations
- Implementing a heartbeat mechanism to extend the lock while streaming is active
- Adding cleanup logic to detect and remove stale locks
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/sim/app/api/a2a/serve/[agentId]/route.ts
Line: 484:499
Comment:
When the streaming lock cannot be acquired (line 486), an error stream is created and returned. However, the lock timeout for streaming is set to 300 seconds (5 minutes, line 484), which is quite long.
If a streaming request hangs or fails without properly releasing the lock, subsequent streaming requests for the same task will be blocked for up to 5 minutes. Consider:
1. Using a shorter lock timeout for streaming operations
2. Implementing a heartbeat mechanism to extend the lock while streaming is active
3. Adding cleanup logic to detect and remove stale locks
<sub>Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!</sub>
How can I resolve this? If you propose a fix, please make it concise.| sessionIdIdx: index('a2a_task_session_id_idx').on(table.sessionId), | ||
| statusIdx: index('a2a_task_status_idx').on(table.status), | ||
| executionIdIdx: index('a2a_task_execution_id_idx').on(table.executionId), | ||
| createdAtIdx: index('a2a_task_created_at_idx').on(table.createdAt), | ||
| }) | ||
| ) | ||
|
|
||
| /** | ||
| * A2A Push Notification Config - Webhook configuration for task updates | ||
| * Stores push notification webhooks for async task updates | ||
| */ |
There was a problem hiding this comment.
The a2aTask table stores messages as JSONB with a default of '[]', but there's no database-level constraint on array size. With A2A_MAX_HISTORY_LENGTH set to 100 in constants.ts, message arrays could grow unbounded if the application-level truncation fails.
Consider:
- Adding application-level cleanup of old messages when updating tasks
- Documenting the expected maximum size for capacity planning
- Adding monitoring for tasks with excessive message counts
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/db/schema.ts
Line: 1892:1902
Comment:
The `a2aTask` table stores `messages` as JSONB with a default of `'[]'`, but there's no database-level constraint on array size. With `A2A_MAX_HISTORY_LENGTH` set to 100 in constants.ts, message arrays could grow unbounded if the application-level truncation fails.
Consider:
1. Adding application-level cleanup of old messages when updating tasks
2. Documenting the expected maximum size for capacity planning
3. Adding monitoring for tasks with excessive message counts
How can I resolve this? If you propose a fix, please make it concise.| export function extractAgentContent(executeResult: { | ||
| output?: { content?: string; [key: string]: unknown } | ||
| error?: string | ||
| }): string { | ||
| return ( | ||
| executeResult.output?.content || | ||
| (typeof executeResult.output === 'object' | ||
| ? JSON.stringify(executeResult.output) | ||
| : String(executeResult.output || executeResult.error || 'Task completed')) | ||
| ) | ||
| } |
There was a problem hiding this comment.
The extractAgentContent function has a fallback chain that could return misleading results. If executeResult.output is an empty object, it will stringify to a string representation instead of using the fallback message.
Consider checking for meaningful content before stringifying objects to avoid returning unhelpful empty object representations.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/sim/app/api/a2a/serve/[agentId]/utils.ts
Line: 139:149
Comment:
The `extractAgentContent` function has a fallback chain that could return misleading results. If `executeResult.output` is an empty object, it will stringify to a string representation instead of using the fallback message.
Consider checking for meaningful content before stringifying objects to avoid returning unhelpful empty object representations.
How can I resolve this? If you propose a fix, please make it concise.
waleedlatif1
force-pushed
the
feat/a2a
branch
from
41a9bab to
db213c7
Compare
| import type { Artifact, Message, PushNotificationConfig, TaskState } from '@a2a-js/sdk' | ||
| import { db } from '@sim/db' | ||
| import { a2aAgent, a2aPushNotificationConfig, a2aTask, workflow } from '@sim/db/schema' | ||
| import { createLogger } from '@sim/logger' |
There was a problem hiding this comment.
Uses relative import @/lib/a2a/constants on line 8 while mixing with @sim/db package imports. Per custom instructions, prefer established path alias patterns consistently throughout the file.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/sim/app/api/a2a/serve/[agentId]/route.ts
Line: 4:4
Comment:
Uses relative import `@/lib/a2a/constants` on line 8 while mixing with `@sim/db` package imports. Per custom instructions, prefer established path alias patterns consistently throughout the file.
<sub>Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!</sub>
How can I resolve this? If you propose a fix, please make it concise.| const history: Message[] = existingTask?.messages ? (existingTask.messages as Message[]) : [] | ||
|
|
||
| history.push(message) | ||
|
|
There was a problem hiding this comment.
Message history arrays in a2aTask.messages grow unbounded. Per A2A_MAX_HISTORY_LENGTH constant (100 messages), consider truncating old messages when history exceeds this limit to prevent unbounded JSONB growth.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/sim/app/api/a2a/serve/[agentId]/route.ts
Line: 331:334
Comment:
Message history arrays in `a2aTask.messages` grow unbounded. Per `A2A_MAX_HISTORY_LENGTH` constant (100 messages), consider truncating old messages when history exceeds this limit to prevent unbounded JSONB growth.
How can I resolve this? If you propose a fix, please make it concise.| return requiresAuth | ||
| ? `import requests | ||
| response = requests.post( | ||
| "${endpoint}", | ||
| headers={ | ||
| "X-API-Key": SIM_API_KEY, | ||
| "Content-Type": "application/json" | ||
| }, | ||
| json=${JSON.stringify(payload, null, 4).replace(/\n/g, '\n ')} | ||
| ) | ||
| print(response.json())` |
There was a problem hiding this comment.
Python code example uses SIM_API_KEY without quotes (line 625), which will fail at runtime. Should be os.environ.get("SIM_API_KEY") or "YOUR_API_KEY" placeholder for a working example.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/deploy/components/deploy-modal/components/a2a/a2a.tsx
Line: 619:631
Comment:
Python code example uses `SIM_API_KEY` without quotes (line 625), which will fail at runtime. Should be `os.environ.get("SIM_API_KEY")` or `"YOUR_API_KEY"` placeholder for a working example.
How can I resolve this? If you propose a fix, please make it concise.| ? `const response = await fetch("${endpoint}", { | ||
| method: "POST", | ||
| headers: { | ||
| "X-API-Key": SIM_API_KEY, | ||
| "Content-Type": "application/json" | ||
| }, | ||
| body: JSON.stringify(${JSON.stringify(payload)}) | ||
| }); | ||
| const data = await response.json(); | ||
| console.log(data);` |
There was a problem hiding this comment.
JavaScript code example uses SIM_API_KEY without quotes (line 647), which will reference an undefined variable. Should be process.env.SIM_API_KEY or "YOUR_API_KEY" placeholder.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/deploy/components/deploy-modal/components/a2a/a2a.tsx
Line: 644:654
Comment:
JavaScript code example uses `SIM_API_KEY` without quotes (line 647), which will reference an undefined variable. Should be `process.env.SIM_API_KEY` or `"YOUR_API_KEY"` placeholder.
How can I resolve this? If you propose a fix, please make it concise.| return requiresAuth | ||
| ? `const response = await fetch("${endpoint}", { | ||
| method: "POST", | ||
| headers: { | ||
| "X-API-Key": SIM_API_KEY, | ||
| "Content-Type": "application/json" | ||
| }, | ||
| body: JSON.stringify(${JSON.stringify(payload)}) | ||
| }); | ||
| const data: Record<string, unknown> = await response.json(); | ||
| console.log(data);` | ||
| : `const response = await fetch("${endpoint}", { |
There was a problem hiding this comment.
TypeScript code example uses SIM_API_KEY without quotes (line 669), which will reference an undefined variable. Should be process.env.SIM_API_KEY or "YOUR_API_KEY" placeholder.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/deploy/components/deploy-modal/components/a2a/a2a.tsx
Line: 665:677
Comment:
TypeScript code example uses `SIM_API_KEY` without quotes (line 669), which will reference an undefined variable. Should be `process.env.SIM_API_KEY` or `"YOUR_API_KEY"` placeholder.
How can I resolve this? If you propose a fix, please make it concise.| // In production, ensure Redis is available for proper distributed locking. | ||
| const lockKey = `a2a:task:${taskId}:lock` | ||
| const lockValue = uuidv4() |
There was a problem hiding this comment.
When Redis is unavailable, acquireLock returns true, allowing concurrent processing without actual locking. Verify that the idempotency layer mentioned in comments is implemented elsewhere to prevent race conditions in multi-replica deployments.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/sim/app/api/a2a/serve/[agentId]/route.ts
Line: 298:300
Comment:
When Redis is unavailable, `acquireLock` returns `true`, allowing concurrent processing without actual locking. Verify that the idempotency layer mentioned in comments is implemented elsewhere to prevent race conditions in multi-replica deployments.
How can I resolve this? If you propose a fix, please make it concise.| function isValidBase64(str: string): boolean { | ||
| if (!str || str.length === 0) return false | ||
| return /^[A-Za-z0-9+/]*={0,2}$/.test(str) | ||
| } | ||
|
|
There was a problem hiding this comment.
Base64 validation accepts empty strings as valid. The length check on line 96 already handles this, but consider if empty base64 data should be treated differently in the file conversion flow.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/sim/lib/a2a/utils.ts
Line: 95:99
Comment:
Base64 validation accepts empty strings as valid. The length check on line 96 already handles this, but consider if empty base64 data should be treated differently in the file conversion flow.
<sub>Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!</sub>
How can I resolve this? If you propose a fix, please make it concise.| if (config.token) { | ||
| headers.Authorization = `Bearer ${config.token}` | ||
| } |
There was a problem hiding this comment.
Tokens are sent as Bearer tokens. Ensure webhook URLs always use HTTPS (validated on line 1104 of serve route) to protect tokens in transit.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/sim/lib/a2a/push-notifications.ts
Line: 43:45
Comment:
Tokens are sent as Bearer tokens. Ensure webhook URLs always use HTTPS (validated on line 1104 of serve route) to protect tokens in transit.
How can I resolve this? If you propose a fix, please make it concise.| ) | ||
|
|
||
| // Require both name and description to publish | ||
| const canSave = name.trim().length > 0 && description.trim().length > 0 |
There was a problem hiding this comment.
Both name AND description are required to publish (canSave on line 302). Consider adding inline validation errors or disabling the publish button with a tooltip explaining what's missing.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/deploy/components/deploy-modal/components/a2a/a2a.tsx
Line: 302:302
Comment:
Both name AND description are required to publish (`canSave` on line 302). Consider adding inline validation errors or disabling the publish button with a tooltip explaining what's missing.
<sub>Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!</sub>
How can I resolve this? If you propose a fix, please make it concise.…, remove custom caching
Summary
Type of Change
Testing
Tested manually
Checklist